use the helm --namespace convention #250
Merged
+249
−300
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Build Failed 😱 Build Id: 8fb85754-3150-418e-bc31-517f06e00156 Build Logs |
|
So my only question about this, is - aren't we also doing stuff in other namespaces - i.e. creating service accounts in the default namespace. Is there any issue about potential confusion/conflict there? Otherwise I'm personally not seeing any issue here. Also, if we are going to do this, we need to also update the Helm install documentation (and maybe have two versions - the 0.2.0 install steps, and then the development install steps, with the standard warning) |
|
Build Succeeded 👏 Build Id: a00d9d6b-fab0-4e7d-afe5-0169540bb3a9 The following development artifacts have been built, and will exist for the next 30 days:
|
|
I'll check if it works when not supplying the --namespace but for sure it's less confusing since right now if you have the |
|
SGTM - then the only outstanding thing for me is an update to the docs. Otherwise, SGTM. |
cyriltovena
force-pushed
the
features/helm-ns
branch
from
d9eca4f to
fb229cb
Compare
|
I've added a small paragrah about --namespace usage. There is no problem with deploying Agones to default as what we're creating for |
|
Build Succeeded 👏 Build Id: 4a0421d9-1212-4f31-9468-df8dae0a71b1 The following development artifacts have been built, and will exist for the next 30 days:
|
markmandel
requested changes
Jun 10, 2018
install/helm/README.md
Outdated
| @@ -21,11 +21,13 @@ Download the latest `agones-install` zip from the [releases](https://github.com/ | |||
| $ wget https://github.com/GoogleCloudPlatform/agones/releases/download/v0.2.0/agones-install-0.2.0.zip | |||
| $ unzip agones-install-0.2.0.zip | |||
| $ cd install/helm/ | |||
| $ helm install --name my-release agones | |||
| $ helm install --name my-release --namespace agones-system agones | |||
There was a problem hiding this comment.
Running this against 0.2.0 results in:
root@markmandel:/go/src/agones.dev/agones# helm install --name test-release --namespace agones-system ./install/helm/agones/
Error: release test-release failed: namespaces "agones-system" already existsI think we'll need to split this - keep the original version for 0.2.0, and then do a development version with the standard
There was a problem hiding this comment.
I feel like this start to be confusing having two type of install. Should we wait then for a release to get that PR in or hotfix 0.2.0 ?
There was a problem hiding this comment.
So I was thinking about something like the following:
https://gist.github.com/markmandel/a4468aa62c6b7793ce36bc398627d402
Two very separate sections -- then when we go to do the 0.3.0 release, we can do our usual documentation cleanup and remove the older install version.
WDYT?
There was a problem hiding this comment.
Done, I've modified your example to make it works with the master branch.
I'm wondering should we have a latest to make it simpler to use the last dev build ? I could then change my doc to use latest.
markmandel
added
kind/documentation
cyriltovena
force-pushed
the
features/helm-ns
branch
from
f2266f2 to
88f4b70
Compare
|
Build Succeeded 👏 Build Id: c903931b-bca0-4f2f-915d-356194574f08 The following development artifacts have been built, and will exist for the next 30 days:
|
…ow helm standard. - (Conflict merge) Update Helm App Version - update doc as requested
cyriltovena
force-pushed
the
features/helm-ns
branch
from
88f4b70 to
24ed1b6
Compare
|
Build Succeeded 👏 Build Id: b378f7a2-f24a-4b7f-8819-cdfc931dca8f The following development artifacts have been built, and will exist for the next 30 days:
|
|
Build Succeeded 👏 Build Id: f9af2915-9be7-4c67-8024-01c8e5b99dbe The following development artifacts have been built, and will exist for the next 30 days:
|
|
Just a heads up - I need to review this. Just been swamped. I'm on it though! |
markmandel
approved these changes
Jun 19, 2018
|
Build Succeeded 👏 Build Id: 2b25eb72-eb99-4210-9e95-e14bc77067f0 The following development artifacts have been built, and will exist for the next 30 days:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The namespace is now created by helm and not our template. This is the convention helm recommend and the one istio uses so helm user won't be lost.
It's also more compatible for use cases when you already have the namespace created.
Tested on GKE !
Closes #212